Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Java: Add support for flow through side-effects on static fields. #16500

Merged
merged 2 commits into from
May 24, 2024
Merged

Java: Add support for flow through side-effects on static fields. #16500

merged 2 commits into from
May 24, 2024

Conversation

aschackmull
Copy link
Contributor

Fixes #15972

@aschackmull aschackmull requested a review from a team as a code owner May 15, 2024 13:40
atorralba
atorralba previously approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@aschackmull
Copy link
Contributor Author

There are a bunch of additional results. The steps provided by this PR look reasonable, but it appears that I'm running into trouble with a bunch of the generated java.time.* models. Are those models reasonable? Or would it make sense to bulk-ditch them? I don't know if they're the primary culprit, but they might be.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 17, 2024 •
edited
Loading

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. A recent commit removed the previously reported differences.

@aschackmull
Copy link
Contributor Author

I've accumulated several different things on this PR now - I'll split the independent changes into separate PRs:
#16550
#16551
#16552

@aschackmull aschackmull mentioned this pull request May 22, 2024
Merged
@aschackmull aschackmull force-pushed the java/static-field-side-effect branch from a40d929 to 1bc3f6b Compare May 23, 2024 11:03
@aschackmull
Copy link
Contributor Author

I've rebased this PR, as the 3 spinoff PRs are now merged, so this PR is back to being nice and simple.

atorralba
atorralba approved these changes May 23, 2024
View reviewed changes
Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm approving exactly the same set of changes as the last time 😄.

@hvitved hvitved mentioned this pull request May 23, 2024
Merged
@aschackmull aschackmull merged commit 5a7174d into github:main May 24, 2024
15 checks passed
@aschackmull aschackmull deleted the java/static-field-side-effect branch May 24, 2024 07:19
owen-mc added a commit to owen-mc/codeql that referenced this pull request May 29, 2024
@owen-mc
Add tests (mostly failing) for writes to global variables
7ff1eab 
This was based on the equivalent for java:
github#16500
@owen-mc owen-mc mentioned this pull request May 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atorralba atorralba atorralba approved these changes

@hvitved hvitved hvitved approved these changes

Assignees
No one assigned
Labels
documentation Java
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cant find taint flow in a LocalVariable statement
3 participants
@aschackmull @hvitved @atorralba